Skip to content

flask to fastapi conversion#188

Merged
ryansurf merged 3 commits into
mainfrom
feat-fastapi
Mar 25, 2026
Merged

flask to fastapi conversion#188
ryansurf merged 3 commits into
mainfrom
feat-fastapi

Conversation

@ryansurf
Copy link
Copy Markdown
Owner

@ryansurf ryansurf commented Mar 24, 2026

General:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Code:

  1. Does your submission pass tests?
  2. Have you run the linter/formatter on your code locally before submission?
  3. Have you updated the documentation/README to reflect your changes, as applicable?
  4. Have you added an explanation of what your changes do?
  5. Have you written new tests for your changes, as applicable?

Summary by Sourcery

Migrate the HTTP server from Flask to FastAPI while preserving existing endpoints and updating tests and dependencies accordingly.

New Features:

  • Expose a FastAPI-based application instance at module level for use with Uvicorn and tests.

Enhancements:

  • Replace Flask server implementation with FastAPI, including CORS middleware configuration and updated error handling for CLI subprocess failures.
  • Adjust template rendering context usage to align with FastAPI/Jinja2 conventions and update environment variable injection in the index template.

Build:

  • Raise the minimum supported Python version to 3.10 and relax the pydantic version constraint.
  • Add FastAPI, Uvicorn, and httpx as runtime dependencies for the project.

Tests:

  • Update server endpoint tests to use FastAPI's TestClient and adapt assertions to the new response interfaces, removing tests for deprecated Flask-only routes.

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Mar 24, 2026

Reviewer's Guide

Migrates the HTTP server implementation from Flask to FastAPI/uvicorn, updates endpoints and tests to use FastAPI semantics and TestClient, adjusts template variable rendering, and refreshes Python/pydantic requirements while adding FastAPI-related dependencies.

Sequence diagram for FastAPI root endpoint invoking CLI

sequenceDiagram
    actor Client
    participant Uvicorn
    participant FastAPIApp
    participant CLIProcess

    Client->>Uvicorn: HTTP GET /
    Uvicorn->>FastAPIApp: Route request
    FastAPIApp->>FastAPIApp: Parse query_params
    FastAPIApp->>FastAPIApp: Build args string
    FastAPIApp->>CLIProcess: subprocess.run(python cli.py args)
    CLIProcess-->>FastAPIApp: stdout or CalledProcessError
    alt CLI success
        FastAPIApp-->>Uvicorn: PlainTextResponse(stdout)
        Uvicorn-->>Client: 200 OK with surf report
    else CLI error
        FastAPIApp-->>Uvicorn: HTTPException 500
        Uvicorn-->>Client: 500 Internal Server Error
    end
Loading

File-Level Changes

Change Details Files
Replace Flask-based server with FastAPI application and uvicorn runner, updating routing, CORS, file responses, error handling, and environment wiring.
  • Remove Flask and flask-cors usage in favor of FastAPI, Jinja2Templates, CORSMiddleware, and FastAPI response classes.
  • Define BASE_DIR, TEMPLATES_DIR, and CLI_PATH constants to build absolute paths for templates, CLI script, and help file.
  • Update /help endpoint to use FastAPI route decorator and FileResponse to return help.txt as plain text.
  • Update root endpoint to FastAPI async handler using Request.query_params instead of Flask request.query_string parsing, and return PlainTextResponse with CLI output.
  • Wrap subprocess errors from the CLI call in HTTPException with 500 status instead of re-raising raw exceptions.
  • Instantiate ServerSettings and FastAPI app at module level and switch main section to run uvicorn instead of app.run().
src/server.py
Adapt tests from Flask test_client to FastAPI TestClient and drop tests for removed Flask-specific endpoints.
  • Remove tests covering /home and /script.js endpoints that no longer exist in the FastAPI server.
  • Replace Flask app.test_client() usages with fastapi.testclient.TestClient for / and /help tests.
  • Adjust test expectations from resp.data (bytes) to resp.text (string) to match TestClient response API.
  • Keep subprocess monkeypatching logic while asserting updated FastAPI HTTP status codes.
tests/test_server.py
tests/test_help_endpoint.py
tests/test_root.py
Update HTML template to align with FastAPI/Jinja2 usage for environment variables while still referencing the static script.
  • Change Jinja expression for env_vars from Flask’s tojson filter to using the safe filter in index.html.
  • Update comment in template to refer to FastAPI server instead of Flask server, while retaining url_for usage for script reference.
src/templates/index.html
Refresh Python and pydantic version constraints and add FastAPI stack dependencies.
  • Bump minimum Python version requirement from >3.9.7 to >=3.10.
  • Relax pydantic version pin to allow compatible 2.x versions via a caret constraint.
  • Add fastapi, uvicorn, and httpx as runtime dependencies for the new FastAPI server and its tests.
pyproject.toml
poetry.lock

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 security issue, 4 other issues, and left some high level feedback:

Security issues:

  • Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)

General comments:

  • The template setup with Jinja2Templates and TEMPLATES_DIR is currently unused and the /home route was removed, while index.html still references env_vars and url_for('serve_script'); either restore a corresponding FastAPI route that renders the template with the expected context or remove/adjust the template and related code to avoid dead/broken paths.
  • Changing {{ env_vars|tojson }} to {{ env_vars | safe }} in index.html removes JSON encoding and may introduce XSS or JS parsing issues if env_vars contains unexpected characters; consider keeping JSON serialization (e.g. via a compatible tojson filter) rather than marking the entire value as safe.
  • Flask and flask-cors remain in pyproject.toml even though the server has been migrated to FastAPI; if Flask is no longer used elsewhere, consider removing those dependencies to keep the dependency set minimal.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The template setup with `Jinja2Templates` and `TEMPLATES_DIR` is currently unused and the `/home` route was removed, while `index.html` still references `env_vars` and `url_for('serve_script')`; either restore a corresponding FastAPI route that renders the template with the expected context or remove/adjust the template and related code to avoid dead/broken paths.
- Changing `{{ env_vars|tojson }}` to `{{ env_vars | safe }}` in `index.html` removes JSON encoding and may introduce XSS or JS parsing issues if `env_vars` contains unexpected characters; consider keeping JSON serialization (e.g. via a compatible `tojson` filter) rather than marking the entire value as safe.
- Flask and flask-cors remain in `pyproject.toml` even though the server has been migrated to FastAPI; if Flask is no longer used elsewhere, consider removing those dependencies to keep the dependency set minimal.

## Individual Comments

### Comment 1
<location path="src/server.py" line_range="54-59" />
<code_context>
+        return FileResponse(path=HELP_FILE_PATH, media_type="text/plain")
+
+
+    @app.get("/", response_class=PlainTextResponse)
+    async def default_route(request: Request):
         """Serves the surf report."""
-        query_parameters = urllib.parse.parse_qsl(
-            request.query_string.decode(), keep_blank_values=True
-        )
         parsed_parameters = [
             f"{key}={value}" if value else key
-            for key, value in query_parameters
+            for key, value in request.query_params.items()
         ]
         args = ",".join(parsed_parameters)
</code_context>
<issue_to_address>
**issue (bug_risk):** Query parameter handling now drops duplicate keys compared to the previous implementation.

`urllib.parse.parse_qsl(..., keep_blank_values=True)` preserved multiple values per key (e.g. `?tag=a&tag=b`), but `request.query_params.items()` only exposes the last value, so duplicates are lost. If the CLI depends on multiple values for a key, this is a behavior change. To keep parity, use an API that preserves duplicates (e.g. `request.query_params.multi_items()`) when building `parsed_parameters`.
</issue_to_address>

### Comment 2
<location path="src/server.py" line_range="43" />
<code_context>
+        "http://127.0.0.1:8501"
+    ]
+
+    app.add_middleware(
+        CORSMiddleware,
+        allow_origins = origins,
+        allow_credentials=True,
+        allow_methods=["GET"],
+        allow_headers=["*"]
+    )
</code_context>
<issue_to_address>
**suggestion (bug_risk):** CORS is restricted to GET only, which may be too tight if non-GET endpoints are introduced.

Compared to the previous `CORS(app)` usage, this configuration will cause any future POST/PUT/DELETE routes to fail CORS preflight even though FastAPI accepts them. If you expect to add non-GET endpoints, consider either listing those methods explicitly or using a broader set of standard methods to avoid hard-to-diagnose frontend errors.

```suggestion
        # Allow standard HTTP methods so future non-GET endpoints won't be blocked by CORS
        allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH"],
```
</issue_to_address>

### Comment 3
<location path="src/templates/index.html" line_range="89-90" />
<code_context>
       </div>
     </div>
     <script>
-      // load .env variables from Flask server
-      const env = {{ env_vars|tojson }};
+      // load .env variables from FastAPI server
+      const env = {{ env_vars | safe }};
     </script>
     <script src="{{ url_for('serve_script') }}"></script>
</code_context>
<issue_to_address>
**🚨 issue (security):** Switching from `tojson` to `safe` risks invalid JavaScript or injection issues.

`tojson` guarantees a valid JS literal and handles quoting/escaping for you. `env_vars | safe` assumes `env_vars` is already a correctly JSON-serialized string and disables escaping. If `env_vars` is a Python dict, it will render invalid JS (e.g. `{'key': 'val'}`) and, if it includes user-controlled data, can introduce XSS. Unless you’re passing a pre-serialized JSON string you fully control, use the FastAPI/Jinja equivalent of `tojson` (or explicit `json.dumps`) instead of `safe`.
</issue_to_address>

### Comment 4
<location path="src/templates/index.html" line_range="92" />
<code_context>
+      // load .env variables from FastAPI server
+      const env = {{ env_vars | safe }};
     </script>
     <script src="{{ url_for('serve_script') }}"></script>
   </body>
</code_context>
<issue_to_address>
**issue (bug_risk):** Template still references `url_for('serve_script')` although the `serve_script` route has been removed.

This reference will now fail at render time, since the Flask `serve_script` route no longer exists. Please either point this to the appropriate FastAPI route (with a matching name) or change it to a static script URL that FastAPI serves correctly.
</issue_to_address>

### Comment 5
<location path="src/server.py" line_range="64-68" />
<code_context>
            result = subprocess.run(
                [sys.executable, str(CLI_PATH), args],
                capture_output=True,
                text=True,
                check=True,
            )
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.

*Source: opengrep*
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/server.py
Comment on lines +54 to +59
@app.get("/", response_class=PlainTextResponse)
async def default_route(request: Request):
"""Serves the surf report."""
query_parameters = urllib.parse.parse_qsl(
request.query_string.decode(), keep_blank_values=True
)
parsed_parameters = [
f"{key}={value}" if value else key
for key, value in query_parameters
for key, value in request.query_params.items()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Query parameter handling now drops duplicate keys compared to the previous implementation.

urllib.parse.parse_qsl(..., keep_blank_values=True) preserved multiple values per key (e.g. ?tag=a&tag=b), but request.query_params.items() only exposes the last value, so duplicates are lost. If the CLI depends on multiple values for a key, this is a behavior change. To keep parity, use an API that preserves duplicates (e.g. request.query_params.multi_items()) when building parsed_parameters.

Comment thread src/server.py
CORSMiddleware,
allow_origins = origins,
allow_credentials=True,
allow_methods=["GET"],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): CORS is restricted to GET only, which may be too tight if non-GET endpoints are introduced.

Compared to the previous CORS(app) usage, this configuration will cause any future POST/PUT/DELETE routes to fail CORS preflight even though FastAPI accepts them. If you expect to add non-GET endpoints, consider either listing those methods explicitly or using a broader set of standard methods to avoid hard-to-diagnose frontend errors.

Suggested change
allow_methods=["GET"],
# Allow standard HTTP methods so future non-GET endpoints won't be blocked by CORS
allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH"],

Comment thread src/templates/index.html
Comment on lines -89 to -90
// load .env variables from Flask server
const env = {{ env_vars|tojson }};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Switching from tojson to safe risks invalid JavaScript or injection issues.

tojson guarantees a valid JS literal and handles quoting/escaping for you. env_vars | safe assumes env_vars is already a correctly JSON-serialized string and disables escaping. If env_vars is a Python dict, it will render invalid JS (e.g. {'key': 'val'}) and, if it includes user-controlled data, can introduce XSS. Unless you’re passing a pre-serialized JSON string you fully control, use the FastAPI/Jinja equivalent of tojson (or explicit json.dumps) instead of safe.

Comment thread src/templates/index.html
// load .env variables from FastAPI server
const env = {{ env_vars | safe }};
</script>
<script src="{{ url_for('serve_script') }}"></script>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Template still references url_for('serve_script') although the serve_script route has been removed.

This reference will now fail at render time, since the Flask serve_script route no longer exists. Please either point this to the appropriate FastAPI route (with a matching name) or change it to a static script URL that FastAPI serves correctly.

Comment thread src/server.py
Comment on lines 64 to 68
result = subprocess.run(
[sys.executable, Path("src") / "cli.py", args],
[sys.executable, str(CLI_PATH), args],
capture_output=True,
text=True,
check=True,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.

Source: opengrep

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/server.py 100.00% <100.00%> (ø)
src/settings.py 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ryansurf ryansurf merged commit e2c98f7 into main Mar 25, 2026
14 checks passed
@ryansurf ryansurf deleted the feat-fastapi branch March 26, 2026 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant